Skip to content

Conversation

@Soar360
Copy link
Contributor

@Soar360 Soar360 commented Jan 16, 2026

本次提交对尺寸字符串解析进行了结构性优化:将 SizeHelper 拆为 partial 并引入多条正则与单位换算字典,提升解析能力和健壮性;新增 Dimension、DimensionKind、DimensionUnit 等结构化类型,为多维度尺寸表达提供基础;新增 Extract 接口(暂未实现),为后续结构化解析做准备。

Soar360 and others added 2 commits January 16, 2026 13:19
本次提交对尺寸字符串解析进行了结构性优化:将 SizeHelper 拆为 partial 并引入多条正则与单位换算字典,提升解析能力和健壮性;新增 Dimension、DimensionKind、DimensionUnit 等结构化类型,为多维度尺寸表达提供基础;新增 Extract 接口(暂未实现),为后续结构化解析做准备。
* Initial plan

* Implement SizeHelper.Extract with dimension parsing - 48/50 tests passing

Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com>

* Fix PerValueUnitPattern to support dimension labels - all 50 tests passing

Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com>

* Fix casing in HEIGHT to Height for consistency

Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com>
@Soar360 Soar360 marked this pull request as ready for review January 16, 2026 06:16
Copilot AI review requested due to automatic review settings January 16, 2026 06:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the size parsing logic by splitting SizeHelper into partial classes and introduces structured types for dimension parsing. The changes enhance the parsing capabilities with support for dimension labels (Width/Height/Length), multiple unit types, and maintain backward compatibility with the existing ExtractSize method.

Changes:

  • Refactored SizeHelper into partial classes with SizeHelper.Dimension.cs containing new structured parsing logic
  • Introduced Dimension, DimensionItem, DimensionKind, and DimensionUnit types for structured dimension representation
  • Added Extract method to parse dimensions with unit and kind information, supporting labels like W/H/L and unit markers
  • Enhanced regex patterns to support dimension labels in the existing PerValueUnitPattern
  • Added comprehensive test coverage for the new Extract method with 18 new test cases

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
src/LuYao.Common/Globalization/SizeHelper.cs Changed to partial class, updated PerValueUnitPattern regex to support dimension labels, minor whitespace cleanup
src/LuYao.Common/Globalization/SizeHelper.Dimension.cs New partial class containing Extract method and related dimension parsing logic with unit detection and conversion
src/LuYao.Common/Globalization/Dimension.cs New record struct DimensionItem and class Dimension for structured dimension representation
src/LuYao.Common/Globalization/DimensionUnit.cs New enum defining supported units (Centimeter, Inch)
src/LuYao.Common/Globalization/DimensionKind.cs New enum defining dimension types (Unspecified, Length, Width, Height)
tests/LuYao.Common.UnitTests/Globalization/SizeHelperTests.cs Added 18 comprehensive test methods for the new Extract method covering various parsing scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +30
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace LuYao.Globalization;

/// <summary>
/// Specifies the type of dimension represented, such as length, width, or height.
/// </summary>
public enum DimensionKind
{
/// <summary>
/// 未指定
/// </summary>
Unspecified,
/// <summary>
/// 长度
/// </summary>
Length,
/// <summary>
/// 宽度
/// </summary>
Width,
/// <summary>
/// 高度
/// </summary>
Height,
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused imports detected. The file only contains an enum definition and doesn't use System, System.Collections.Generic, System.Linq, System.Text, or System.Threading.Tasks. Remove these unused imports to keep the code clean.

Suggested change
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace LuYao.Globalization;
/// <summary>
/// Specifies the type of dimension represented, such as length, width, or height.
/// </summary>
public enum DimensionKind
{
/// <summary>
/// 未指定
/// </summary>
Unspecified,
/// <summary>
/// 长度
/// </summary>
Length,
/// <summary>
/// 宽度
/// </summary>
Width,
/// <summary>
/// 高度
/// </summary>
Height,
}
namespace LuYao.Globalization;
/// <summary>
/// Specifies the type of dimension represented, such as length, width, or height.
/// </summary>
public enum DimensionKind
{
/// <summary>
/// 未指定
/// </summary>
Unspecified,
/// <summary>
/// 长度
/// </summary>
Length,
/// <summary>
/// 宽度
/// </summary>
Width,
/// <summary>
/// 高度
/// </summary>
Height,
}

Copilot uses AI. Check for mistakes.
using System.Collections.Generic;

namespace LuYao.Globalization;

Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Public API type DimensionItem lacks XML documentation. Add a summary comment explaining what a DimensionItem represents (e.g., 'Represents a single dimension value with its associated kind').

Suggested change
/// <summary>
/// Represents a single dimension value together with its associated kind.
/// </summary>

Copilot uses AI. Check for mistakes.
Comment on lines +4 to +10

public record struct DimensionItem(DimensionKind Kind, decimal Value);

public class Dimension
{
public DimensionUnit Unit { get; set; } = DimensionUnit.Centimeter;
public List<DimensionItem> Items { get; set; } = new List<DimensionItem>();
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Public API class Dimension and its properties lack XML documentation. Add summary comments to explain the purpose of this class and each property.

Suggested change
public record struct DimensionItem(DimensionKind Kind, decimal Value);
public class Dimension
{
public DimensionUnit Unit { get; set; } = DimensionUnit.Centimeter;
public List<DimensionItem> Items { get; set; } = new List<DimensionItem>();
/// <summary>
/// Represents a single dimension value along with its associated kind.
/// </summary>
/// <param name="Kind">The kind or category of the dimension value.</param>
/// <param name="Value">The numeric value of the dimension.</param>
public record struct DimensionItem(DimensionKind Kind, decimal Value);
/// <summary>
/// Represents a collection of dimension measurements expressed in a specific unit.
/// </summary>
public class Dimension
{
/// <summary>
/// Gets or sets the unit of measurement used for the dimension items.
/// </summary>
public DimensionUnit Unit { get; set; } = DimensionUnit.Centimeter;
/// <summary>
/// Gets or sets the list of dimension items that compose this dimension.
/// </summary>
public List<DimensionItem> Items { get; set; } = new List<DimensionItem>();
/// <summary>
/// Gets a value indicating whether the dimension contains no items.
/// </summary>

Copilot uses AI. Check for mistakes.
Comment on lines +730 to +737
public void Extract_MixedUnitsWithLabels_ParsesCorrectly()
{
var result = SizeHelper.Extract("10cmW x 20''H");

Assert.AreEqual(1, result.Count);
var dimension = result[0];
// 当有多种单位时,优先检测到的单位作为主单位
Assert.AreEqual(2, dimension.Items.Count);
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test doesn't verify the actual unit of the dimension when mixed units are present (10cmW x 20''H contains both cm and inches). The comment indicates 'the first detected unit becomes the primary unit' but the test doesn't assert which unit was chosen. Add assertion for dimension.Unit to verify the expected behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +730 to +741
public void Extract_MixedUnitsWithLabels_ParsesCorrectly()
{
var result = SizeHelper.Extract("10cmW x 20''H");

Assert.AreEqual(1, result.Count);
var dimension = result[0];
// 当有多种单位时,优先检测到的单位作为主单位
Assert.AreEqual(2, dimension.Items.Count);

Assert.AreEqual(DimensionKind.Width, dimension.Items[0].Kind);
Assert.AreEqual(DimensionKind.Height, dimension.Items[1].Kind);
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test doesn't verify the actual values of the dimension items when mixed units are present. If the values are being converted to a common unit, the test should assert the expected converted values to ensure the conversion logic is working correctly.

Copilot uses AI. Check for mistakes.
Comment on lines +169 to +217
foreach (var part in parts)
{
if (string.IsNullOrWhiteSpace(part))
continue;

var trimmed = part.Trim();

// 解析单个部分: 数字 + 可选英寸标记 + 可选维度标签 + 可选单位 + 可选维度标签
// 英寸标记可以是: '', ", '
var match = System.Text.RegularExpressions.Regex.Match(trimmed,
@"^(\d+(?:\.\d+)?)\s*(''|[''""])?\s*([WwHhLl]|Width|Height|Length)?\s*(inch|in|mm|cm|dm|m)?\s*([WwHhLl]|Width|Height|Length)?$",
System.Text.RegularExpressions.RegexOptions.IgnoreCase);

if (!match.Success || !match.Groups[1].Success)
continue;

if (!decimal.TryParse(match.Groups[1].Value, out decimal value))
continue;

// 检测英寸标记 (group 2)
bool hasInchMarker = match.Groups[2].Success && !string.IsNullOrEmpty(match.Groups[2].Value);

// 检测单位 (group 4)
string unitStr = match.Groups[4].Success ? match.Groups[4].Value : "";

// 检测维度标签 (group 3 在单位前, group 5 在单位后)
// 优先使用单位后的标签
string labelStr = "";
if (match.Groups[5].Success && !string.IsNullOrEmpty(match.Groups[5].Value))
{
labelStr = match.Groups[5].Value;
}
else if (match.Groups[3].Success && !string.IsNullOrEmpty(match.Groups[3].Value))
{
labelStr = match.Groups[3].Value;
}

// 转换值到目标单位
decimal convertedValue = ConvertValueToUnit(value, unitStr, hasInchMarker, targetUnit);

// 解析维度类型
DimensionKind kind = DimensionKind.Unspecified;
if (!string.IsNullOrEmpty(labelStr))
{
kind = ParseDimensionKind(labelStr);
}

items.Add(new DimensionItem(kind, convertedValue));
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot uses AI. Check for mistakes.
Comment on lines +241 to +264
foreach (System.Text.RegularExpressions.Match match in matches)
{
if (!match.Groups[1].Success || string.IsNullOrWhiteSpace(match.Groups[1].Value))
continue;

if (!decimal.TryParse(match.Groups[1].Value, out decimal value))
continue;

if (value == 0)
continue;

// 转换值到目标单位
bool hasInchMarker = group.Contains("''") || group.Contains("\"");
decimal convertedValue = ConvertValueToUnit(value, detectedUnitStr, hasInchMarker, targetUnit);

// 检测维度类型
DimensionKind kind = DimensionKind.Unspecified;
if (match.Groups[2].Success && !string.IsNullOrEmpty(match.Groups[2].Value))
{
kind = ParseDimensionKind(match.Groups[2].Value);
}

items.Add(new DimensionItem(kind, convertedValue));
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot uses AI. Check for mistakes.
Comment on lines +85 to +94
if (hasPerValueUnit)
{
// 每个值有自己的单位
dimension.Items = ParseWithPerValueUnits(group, dimension.Unit);
}
else
{
// 所有值共享一个单位
dimension.Items = ParseWithSingleUnit(group, dimension.Unit);
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

Suggested change
if (hasPerValueUnit)
{
// 每个值有自己的单位
dimension.Items = ParseWithPerValueUnits(group, dimension.Unit);
}
else
{
// 所有值共享一个单位
dimension.Items = ParseWithSingleUnit(group, dimension.Unit);
}
dimension.Items = hasPerValueUnit
? ParseWithPerValueUnits(group, dimension.Unit)
: ParseWithSingleUnit(group, dimension.Unit);

Copilot uses AI. Check for mistakes.
Comment on lines +277 to +284
if (targetUnit == DimensionUnit.Inch)
{
return value; // 保持英寸
}
else
{
return value * 2.54m; // 转换为厘米
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both branches of this 'if' statement return - consider using '?' to express intent better.

Copilot uses AI. Check for mistakes.
Comment on lines +319 to +326
if (targetUnit == DimensionUnit.Inch)
{
return value;
}
else
{
return value * 2.54m;
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both branches of this 'if' statement return - consider using '?' to express intent better.

Copilot uses AI. Check for mistakes.
@Soar360 Soar360 merged commit d4c2bc0 into main Jan 16, 2026
6 of 7 checks passed
@Soar360 Soar360 deleted the feat/enhance-sizehelper-functionality branch January 16, 2026 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants